Skip to content

+TCK verifyNoAsyncErrors now by default waits, fixes spec111 #240

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

ktoso
Copy link
Contributor

@ktoso ktoso commented Mar 11, 2015

Resolves #239

Don't merge yet, I want to look though all other specs but lacked the capacity to have done this today.
Question that needs feedback here is if we want to rename the existing method to be less confusing - I say yes.

* Waits for {@link TestEnvironment#defaultTimeoutMillis()} and then verifies that no asynchronous errors
* were signalled pior to, or during that time (by calling {@code flop()}).
*/
public void verifyNoAsyncErrors() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this ought to call verifyNoAsyncErrors(defaultTimeoutMillis())

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeap, totally true.

@viktorklang
Copy link
Contributor

@ktoso Reviewed. What's the status?

@ktoso ktoso force-pushed the async-errors-with-timeout branch from 9453fd3 to 4df0d4d Compare March 14, 2015 00:32
@ktoso ktoso changed the title [[WIP] Don't merge yet] +TCK verifyNoAsyncErrors now by default waits, fixes spec111 [WIP] +TCK verifyNoAsyncErrors now by default waits, fixes spec111 Mar 16, 2015
@viktorklang
Copy link
Contributor

@ktoso is this still WIP, and if so, when will it not be WIP anymore?

@ktoso
Copy link
Contributor Author

ktoso commented Mar 24, 2015

The WIP part still exists, but it is (possibly) additive, not other in-another-way changing (maybe one or two more tests needs have delay but don't). Since that test we identified as troublesome let's fix it.

I propose to merge as is and if I find another instance of such problem it's a good stand-along PR.

@viktorklang
Copy link
Contributor

@ktoso I won't merge something that is WIP so split the WIP part out into another Issue/PR. Thanks!

@ktoso ktoso changed the title [WIP] +TCK verifyNoAsyncErrors now by default waits, fixes spec111 +TCK verifyNoAsyncErrors now by default waits, fixes spec111 Mar 24, 2015
@ktoso
Copy link
Contributor Author

ktoso commented Mar 24, 2015

The WIP part is "look at more tests with intense passion", no actual code in the PR.
I need to rebase anyway, I'll do that just after I get some time today.

@viktorklang
Copy link
Contributor

@ktoso Thanks, Konrad, I appreciate it!

@ktoso ktoso force-pushed the async-errors-with-timeout branch 2 times, most recently from d7a2e1d to 2512dc4 Compare March 25, 2015 00:24
@ktoso
Copy link
Contributor Author

ktoso commented Mar 25, 2015

Rebased, re-read the diff and checked against akka that it fixes the discovered problem.
This one is ready to merge :-)

@ktoso ktoso force-pushed the async-errors-with-timeout branch from 2512dc4 to 0b055a5 Compare March 25, 2015 00:28
@ktoso
Copy link
Contributor Author

ktoso commented Mar 25, 2015

Also included the link fix we mentioned during review (reactive-streams/reactive-streams => reactive-streams/reactive-streams-jvm)

Resolves #249

@viktorklang
Copy link
Contributor

👍 great work Konrad, thank you!

@reactive-streams/contributors Merging.

viktorklang added a commit that referenced this pull request Mar 25, 2015
+TCK verifyNoAsyncErrors now by default waits, fixes spec111
@viktorklang viktorklang merged commit 2fe7c48 into reactive-streams:master Mar 25, 2015
@viktorklang viktorklang deleted the async-errors-with-timeout branch March 25, 2015 10:01
@viktorklang viktorklang modified the milestone: 1.0.0.RC4 Mar 25, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

optional_spec111_maySupportMultiSubscribe has no delay before checking async errors
2 participants